feat: new position details on crvusd/lend #1097
Conversation
…m/curvefi/curve-frontend into feat/new-lending-position-details
…m/curvefi/curve-frontend into feat/new-lending-position-details
There was a problem hiding this comment.
Lots of comments and wrok that needs to be done, but shouldn't be too much of an issue as the new feature is behind a beta flag; can improve in new PRs.
Some of my comments also apply to other areas as well, given there's some code duplication going on wrt the loan and lend apps.
| marginTop: Spacing.xl, | ||
| marginBottom: Spacing.xxl, | ||
| gap: Spacing.xl, | ||
| flexDirection: 'column', |
There was a problem hiding this comment.
Stack is column in direction by default, prob not needed
| gap: Spacing.xl, | ||
| flexDirection: 'column', | ||
| // 960px | ||
| '@media (min-width: 60rem)': { |
There was a problem hiding this comment.
[theme.breakpoints.down('desktop')]: {
flexDirection: 'row'
},or, in useBreakpoints.ts:
const isDesktopUp = (theme: Theme) => theme.breakpoints.down('desktop')
export const useIsDesktopDown = () => useMediaQuery(isDesktopDown)
<Stack direction={useIsDesktopDown() ? 'row' : 'column'}> </Stack>960px should be close enough to desktop breakpoint.
Point is, hardcoding media stuff like this should be avoided, esp sizes.
| gap: Spacing.xl, | ||
| flexDirection: 'column', | ||
| // 960px | ||
| '@media (min-width: 60rem)': { |
| gap: Spacing.xl, | ||
| flexDirection: 'column', | ||
| // 960px | ||
| '@media (min-width: 60rem)': { |
There was a problem hiding this comment.
Same as page loan create numero dos
| const [selectedBand, setSelectedBand] = useState<'user' | 'market'>(page === 'create' ? 'market' : 'user') | ||
|
|
||
| const SelectorMenu = | ||
| page === 'create' ? null : ( |
There was a problem hiding this comment.
page && (<SelectorRow
Although I'm not sure, might show false lol
|
|
||
| return ( | ||
| <Box sx={{ display: 'flex', flexDirection: 'column', gap: LABEL_GAP }} paddingBottom="0.3125rem"> | ||
| <Box sx={{ display: 'flex', position: 'relative', width: '100%', height: '1rem' }}> |
There was a problem hiding this comment.
<Stack direction="row">
| export const HealthDetails = ({ health }: { health: Health }) => { | ||
| const theme = useTheme() | ||
| return ( | ||
| <Box |
There was a problem hiding this comment.
<Stack>, in multiple places here
| label={t`Health`} | ||
| value={Number(health?.value)} | ||
| loading={health?.loading} | ||
| valueOptions={{ unit: 'percentage', decimals: 2, color: getHealthValueColor(health?.value ?? 0, theme) }} |
There was a problem hiding this comment.
If you inline useTheme() here you can use arrow-body notation and reduce indenting by a level 🤓
| {t`Health determines a position liquidation. It is not directly correlated to the price of the collateral. `} | ||
| </Typography> | ||
| <Typography variant="bodyXsRegular" sx={{ fontWeight: (t) => t.typography.fontWeightBold }}> | ||
| {t`Liquidations occur when health reaches 0.`} |
There was a problem hiding this comment.
Liquidations may occur 🤓
| loading={borrowAPY?.value == null && borrowAPY?.loading} | ||
| valueOptions={{ unit: 'percentage' }} | ||
| notional={ | ||
| borrowAPY?.thirtyDayAvgRate |
There was a problem hiding this comment.
Not just here, but in a lot of <Metric /> usages in this PR you use a ternary in notionals. I think you can get away using the && operator instead?
borrowAPY?.thirtyDayAvgRate && { value: borrowAPY.thirtyDayAvgRate, unit: { symbol: '% 30D Avg', position: 'suffix' }}
This PR introduces new views market/position details for /create, /manage and /vault on the crvusd and lend market pages. They are available when BETA features are enabled.
Changes: